Skip to content

Conditionally compute visual line layout in Thinking component - #1218

Open
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:perf-thinking-conditional-layout
Open

Conditionally compute visual line layout in Thinking component#1218
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:perf-thinking-conditional-layout

Conversation

@nordicnode

Copy link
Copy Markdown

Conditionally compute visual line layout in Thinking component

Summary

• In cli/src/components/thinking.tsx, conditionally compute visual line wrapping and markdown content formatting based on thinkingCollapseState.
• Previously, getLastNVisualLines (which measures tokens and performs visual word wrapping across the entire reasoning text) was invoked unconditionally on every render, even when thinkingCollapseState === 'hidden' (where neither preview nor expanded text is displayed) or 'expanded' (where preview lines are never displayed).
• Measured via @opentui/react and @opentui/core/testing end-to-end across 100 actual React render cycles (5,000-char reasoning text):

  • Hidden state: 753.35 ms down to 148.99 ms (5.05x faster end-to-end React render).
  • Expanded state: 779.28 ms down to 181.15 ms (4.30x faster end-to-end React render).
  • Isolated visual wrapping: 616.26 ms down to 0.00 ms.
    • In cli/src/components/blocks/thinking-block.tsx, added a fast-path for blocks.length === 1 when extracting combinedContent to avoid allocating intermediate .map() arrays on every streaming token update.
    • Added unit tests in cli/src/components/__tests__/thinking.test.tsx verifying component rendering across all three states (preview, hidden, and expanded).

Test plan

[✓] bun test --config=/dev/null src/components/__tests__/thinking.test.tsx — 3 pass, 0 fail
[✓] bun run --cwd cli typecheck — passed with 0 errors
[✓] PR hygiene check passed

@codebuff-team

Copy link
Copy Markdown
Contributor

Nice, focused change. The core insight—that getLastNVisualLines and the expanded-content normalization were being computed unconditionally even when hidden (neither is shown) or expanded (preview isn't shown)—is legitimate and the fix in thinking.tsx preserves the original render output for all three states (I traced showPreview/showFull through the new branches and they match the old semantics: hidden computes neither, expanded skips the preview-line wrap, preview skips the expanded-markdown normalization).

The thinking-block.tsx fast-path for blocks.length === 1 is a reasonable micro-optimization to avoid a throwaway array from .map() on every streaming token, though its actual impact is probably marginal compared to the wrapping fix — that's fine, it's still correct and harmless.

Good to see unit tests added for all three collapse states (preview, hidden, expanded) verifying the rendered frame contents rather than just internal state, which is the right level to test a terminal UI component at.

A couple of things worth double checking before this lands: (1) confirm getLastNVisualLines and expandedContent truly have no side effects relied on elsewhere (e.g. some parent measuring width off lines) — a quick grep of call sites would settle this. (2) The benchmark numbers in the PR description are useful context but aren't part of the diff/test suite, so they're not verifiable in review — that's fine for a description but the correctness argument stands independently.

Small, in-scope, testable — worth porting.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants